fix(provenance): require textual identity locators - #287
Conversation
|
🔴 Contributor Check: HIGH
Automated check by AgenTrust Contributor Check. |
github-actions
Bot
added
the
needs-review:HIGH
lywinged
left a comment
lywinged
left a comment
There was a problem hiding this comment.
Approved on c943434 over fc38496. Reproduced on main: _check_structure() tests artifact.package and endpoint.url by truthiness, so true, 1, [1] and {"x": 1} satisfy the presence check. On this head each is refused through both build_record() and verify_record() and textual locators pass; the PR's own test file has 16 of 17 cases failing against main and none here. Suite 1,184 passed, 1 skipped; ruff, mypy and the dash check clean.
Tool-assisted: the runs and this write-up.
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Review/test follow-up on c9434342c494.
Identity locators now require textual values rather than accepting truthy objects or numbers. Earlier focused validation passed 17 cases; 16 failed against the old implementation. The full hosted CI rerun passed. No additional blocker found in this change.
Full local python -m pytest -q (Windows, Python 3.12, existing review environment): 5 failed, 1179 passed, 1 skipped in 10.49s.
The five local failures are the four generated-fixture byte comparisons and test_every_schema_in_the_repository_is_classified, matching the Windows newline/path-separator failures seen across these branches. The hosted Linux suite is green; the local suite is not.
Contribution requirement still outstanding: these commits have no Signed-off-by: trailer: cd5343d3, c9434342. Please have the respective commit authors add their own DCO sign-offs, including merge commits, then request review of the resulting head.
Fresh workflow evidence: 33886018121 (success).
Signed-off-by: Altru.dev <altrudevelop@gmail.com>
Signed-off-by: Altru.dev <altrudevelop@gmail.com>
c943434 to
cdcb61c
Compare
3 participants
Closes #285.
What
spec/server-provenance-v1.mddefinesidentity.artifact.packageas a Package URL andidentity.endpoint.urlas a URL. The shared producer/consumer structural check previously used truthiness only, so truthy non-string JSON values could satisfy both locator-presence checks.This change adds one small non-empty-string boundary and applies it to both locator fields from
_check_structure(), which is shared bybuild_record()andverify_record().Regression coverage
For both producer and verifier paths, the tests refuse:
true;1;[1];{\"x\": 1}.Positive controls keep ordinary textual Package URL and HTTPS locator values working.
Scope
This is intentionally only the primitive/type boundary. It does not add full Package URL parsing or broader URL syntax policy.
AI-assistance disclosure: ChatGPT assisted with source triage, adversarial-case design, implementation drafting, and diff review.
altrudevreviewed the bounded claim and remains responsible for the contribution.